view: impl Serialize for generated view types (#83)#106
Open
tejas-dharani wants to merge 1 commit intoanthropics:mainfrom
Open
view: impl Serialize for generated view types (#83)#106tejas-dharani wants to merge 1 commit intoanthropics:mainfrom
tejas-dharani wants to merge 1 commit intoanthropics:mainfrom
Conversation
…ics#83) When `generate_json` is enabled, each generated `*View<'a>` struct now gets a manual `impl<'__a> serde::Serialize for FooView<'__a>` that follows proto3-JSON semantics: proto3 defaults omitted, bytes base64-encoded, int64/uint64 quoted, NaN/Inf as string tokens, enum values as proto names, NullValue oneof variants as JSON null. `OwnedView<V>` gains a blanket `impl<V: Serialize> Serialize for OwnedView<V>` (cfg-gated on `feature = "json"`) so that `serde_json::to_string(&owned_view)` works without an explicit deref. Known limitations (documented in CHANGELOG and generated impl doc): - Messages whose views nest a WKT view (TimestampView, DurationView, etc.) fail to compile — WKT view Serialize is a planned follow-up. - Extension fields are not included in view JSON output. Fixes: anthropics#83
|
All contributors have signed the CLA ✍️ ✅ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #83.
View types had no JSON serialization path. Users holding a decoded
FooView<'_>had to call.to_owned_message()and allocate a full owned message just to serialize to JSON — the whole point of views is to avoid that allocation.Problem
generate_jsonalready wired up#[derive(Serialize, Deserialize)]on owned message structs and enums. Views were left out because they require a manualimplrather than a derive: field access goes through getter methods, map fields need a custom iterator wrapper, and the proto3 JSON rules (omit defaults, quote int64/uint64, base64-encode bytes, encode NaN/Inf as string tokens, NullValue as JSONnull) need explicit handling at codegen time. There was nogenerate_view_serializefunction inbuffa-codegen/src/view.rs.Fix
Added
generate_view_serializeinbuffa-codegen/src/view.rs. When bothgenerate_viewsandgenerate_jsonare enabled for a file, each generated*View<'a>struct gets a manualimpl<'__a> serde::Serialize for FooView<'__a>that opens a serde map, iterates fields, and applies the same proto3 JSON rules as the owned-side impl: proto3 defaults are skipped, bytes go throughjson_helpers::bytes_serialize, int64/uint64 throughjson_helpers::int64_serialize, enum values emit their proto name, and NullValue oneof variants emit JSONnullviaserialize_entry(key, &()).Map fields use a local
_WMnewtype that iterates theMapViewand calls the appropriate key/value helpers inside aSerializeMap; the pattern mirrors the existing owned-message approach. Scalar fields needing a helper emit a local_Wnewtype inside a block scope — this scope is required when a message has more than one helper-typed required field (proto2), because block scoping prevents the duplicate-definition error that would otherwise result from twostruct _Wdeclarations at function scope.OwnedView<V>gains a blanketimpl<V: serde::Serialize> serde::Serialize for OwnedView<V>(cfg-gated onfeature = "json") inbuffa/src/view.rs, soserde_json::to_string(&owned_view)works without an explicit&*owned_viewderef.Known limitations (documented in CHANGELOG and in the generated impl's doc comment): messages whose view types nest a WKT view (
TimestampView,DurationView, etc.) fail to compile because WKT view types do not yet implementSerialize. The workaround is to call.to_owned_message()and serialize the owned form; fixing the WKT side is a planned follow-up. Extension fields are also not included in view JSON output.Tests
Four codegen integration tests cover the code-generation path: one verifies the
impl serde::Serializeblock is present whengenerate_jsonis enabled, one verifies it is absent when disabled, one checks thatjson_helperscalls appear for bytes and int64 fields, and one is a regression test for the proto2 required-field_W-collision fix (verifies the emitted token stream parses withoutE0428).Nine round-trip tests in
buffa-test/src/tests/json.rsexercise the generated impls end-to-end againstview_json.proto(a dedicated proto file with no WKT message imports): scalar fields including int64 quoting and bytes base64, double special values (NaN/Inf/-Inf), proto3 default omission, enum name encoding, all oneof variants including NullValue, map fields with int32 keys and int64 values, nested messages with repeated fields, and theOwnedView<V>blanket impl. Each test decodes a wire-encoded message into a view and comparesserde_json::to_string(&view)againstserde_json::to_string(&owned)to confirm parity.